Skip to content

feat(sdk/go)!: replace positional labels with functional Create options - #3198

Open
rhuss wants to merge 1 commit into
NVIDIA:mainfrom
rhuss:6117-go-sdk-create-options
Open

feat(sdk/go)!: replace positional labels with functional Create options#3198
rhuss wants to merge 1 commit into
NVIDIA:mainfrom
rhuss:6117-go-sdk-create-options

Conversation

@rhuss

@rhuss rhuss commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Unify the inconsistent treatment of labels (positional parameter) and annotations (CreateOptions struct field) by moving both into functional options on SandboxInterface.Create().

Before:

Create(ctx, ws, name, spec, labels, CreateOptions{Annotations: ann})
Create(ctx, ws, name, nil, nil)  // unclear what the nils mean

After:

Create(ctx, ws, name, spec, WithLabels(labels), WithAnnotations(ann))
Create(ctx, ws, name, nil)  // clean, no trailing nils

Related Issue: Closes #2807

Changes

  • Added CreateOption functional option type following the existing LogOption pattern
  • Added WithLabels() and WithAnnotations() option functions
  • Removed the old CreateOptions struct (clean break, SDK is pre-1.0)
  • Updated SandboxInterface.Create() signature
  • Updated real client, fake client, and all ~35 call sites across tests and docs

Testing

  • All Go SDK tests pass (go test ./...)
  • go build ./... and go vet ./... clean
  • Full pre-commit suite passes

Checklist

  • Tests pass
  • Breaking change documented in commit message
  • Documentation updated (README, getting-started, error-handling, testing, sandboxes, fake docs)
  • Signed off (DCO)

@copy-pr-bot

copy-pr-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@rhuss
rhuss force-pushed the 6117-go-sdk-create-options branch 2 times, most recently from 4d0ad25 to ee2edef Compare September 5, 2026 13:29
Move labels and annotations from separate parameter positions into
functional options (WithLabels, WithAnnotations) on Create() and
CreateFromTemplate().

Before: Create(ctx, ws, name, spec, labels, CreateOptions{Annotations: ann})
After:  Create(ctx, ws, name, spec, WithLabels(labels), WithAnnotations(ann))

This unifies two resource metadata fields that were inconsistently
placed (labels as positional, annotations in a struct) into a single
extensible options pattern matching the existing LogOption convention.

BREAKING CHANGE: Create() and CreateFromTemplate() signatures changed.
All callers must use WithLabels() and WithAnnotations() functional
options instead of positional labels and CreateOptions struct.

Closes NVIDIA#2807

Signed-off-by: Roland Huß <rhuss@redhat.com>
@rhuss
rhuss force-pushed the 6117-go-sdk-create-options branch from ee2edef to c6506d7 Compare September 5, 2026 13:30
@rhuss

rhuss commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@elezar Here's a first try to streamline the creation signature a bit more. For a more in depth discussion see my comments on #2807

@elezar elezar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing callers using Create(ctx, ws, name, spec, nil) for an unlabeled sandbox still compile after this API change: nil is a valid CreateOption value. ApplyCreateOptions then invokes that nil function and panics at runtime. Please ignore nil options in ApplyCreateOptions (and add a regression test covering the legacy trailing-nil call) so this migration remains safe for existing consumers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(sdk/go): bundle Create() positional parameters into options struct

2 participants